-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jpeg2000: valid_file implementation, much faster than trying to open #4548
Conversation
Primary cost of detecting whether a given file is valid jpeg2000 file is the thread pool initialization and shutdown that Jpeg2000Input::open does (the actual thread pool part is inside OpenJpeg code). So add a dedicated valid_file() implementation that only needs to check 12 bytes of the header. While at it, I changed already existing isJp2File() to is_jp2_header() to better match naming conventions used elsewhere, and instead of trying to handle both little and big endian cases by manual repetition of two sets of magic integers, let's do just byte comparisons with memcmp instead. On my PC (Ryzen 5950X, SSD, Windows VS2022), doing ImageInput::create() on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them): - Before: 3.4 seconds spent in Jpeg2000Input::open (1.9s opj_thread_pool_create, 1.3s opj_thread_pool_destroy) - After: 33 milliseconds spent in Jpeg2000Input::valid_file Signed-off-by: Aras Pranckevicius <aras@nesnausk.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great find!
(Note to self, or anybody else reading: I wonder if the openjpeg library has a facility to the caller to provide its own thread pool instead of having a new one created and potentially over-threading.)
Oof, the OpenJPEG library creates and destroys a threadpool for every file it reads (or tries to read)? It really doesn't just let the thread pool persist so that if multiple files are read, it doesn't do a full build/teardown each time? |
Always good to see you helping out here, @aras-p. You never fail to turn up some really interesting thing that we had previously overlooked. |
Looks like it. It's... not a design I would recommend, let's put it that way :) |
…open (AcademySoftwareFoundation#4548) Primary cost of detecting whether a given file is valid jpeg2000 file is the thread pool initialization and shutdown that `Jpeg2000Input::open` does (the actual thread pool part is inside OpenJpeg code). So add a dedicated `valid_file()` implementation that only needs to check 12 bytes of the header. While at it, I changed already existing `isJp2File()` to `is_jp2_header()` to better match naming conventions used elsewhere, and instead of trying to handle both little and big endian cases by manual repetition of two sets of magic integers, let's do just byte comparisons with `memcmp` instead. On my PC (Ryzen 5950X, SSD, Windows VS2022), doing `ImageInput::create()` on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them): - Before: **3.4 seconds** spent in `Jpeg2000Input::open` (1.9s `opj_thread_pool_create`, 1.3s `opj_thread_pool_destroy`) - After: **33 milliseconds** spent in `Jpeg2000Input::valid_file` No new tests. I checked behavior on the official Jpeg2000 conformance data set (https://github.com/uclouvain/openjpeg-data/tree/master/input/conformance), seems to work. Signed-off-by: Aras Pranckevicius <aras@nesnausk.org>
…open (AcademySoftwareFoundation#4548) Primary cost of detecting whether a given file is valid jpeg2000 file is the thread pool initialization and shutdown that `Jpeg2000Input::open` does (the actual thread pool part is inside OpenJpeg code). So add a dedicated `valid_file()` implementation that only needs to check 12 bytes of the header. While at it, I changed already existing `isJp2File()` to `is_jp2_header()` to better match naming conventions used elsewhere, and instead of trying to handle both little and big endian cases by manual repetition of two sets of magic integers, let's do just byte comparisons with `memcmp` instead. On my PC (Ryzen 5950X, SSD, Windows VS2022), doing `ImageInput::create()` on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them): - Before: **3.4 seconds** spent in `Jpeg2000Input::open` (1.9s `opj_thread_pool_create`, 1.3s `opj_thread_pool_destroy`) - After: **33 milliseconds** spent in `Jpeg2000Input::valid_file` No new tests. I checked behavior on the official Jpeg2000 conformance data set (https://github.com/uclouvain/openjpeg-data/tree/master/input/conformance), seems to work. Signed-off-by: Aras Pranckevicius <aras@nesnausk.org>
Description
Primary cost of detecting whether a given file is valid jpeg2000 file is the thread pool initialization and shutdown that
Jpeg2000Input::open
does (the actual thread pool part is inside OpenJpeg code).So add a dedicated
valid_file()
implementation that only needs to check 12 bytes of the header. While at it, I changed already existingisJp2File()
tois_jp2_header()
to better match naming conventions used elsewhere, and instead of trying to handle both little and big endian cases by manual repetition of two sets of magic integers, let's do just byte comparisons withmemcmp
instead.On my PC (Ryzen 5950X, SSD, Windows VS2022), doing
ImageInput::create()
on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them):Jpeg2000Input::open
(1.9sopj_thread_pool_create
, 1.3sopj_thread_pool_destroy
)Jpeg2000Input::valid_file
Tests
No new tests. I checked behavior on the official Jpeg2000 conformance data set (https://github.com/uclouvain/openjpeg-data/tree/master/input/conformance), seems to work.
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.